Skip to content

Remove local strings from timing - #12513

Open
simonrozsival wants to merge 11 commits into
dev/simonrozsival/use-join-paths-for-loggerfrom
dev/simonrozsival/remove-timing-dynamic-strings
Open

Remove local strings from timing#12513
simonrozsival wants to merge 11 commits into
dev/simonrozsival/use-join-paths-for-loggerfrom
dev/simonrozsival/remove-timing-dynamic-strings

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Aug 25, 2026

Copy link
Copy Markdown
Member

Summary

Remove the shared fast-timing implementation's local-string dependencies without introducing limits based on their former inline-storage thresholds.

Changes

  • keep fixed arrays only for genuinely bounded Android properties, logcat messages, and integer formatting
  • make assembly timing metadata formatting return the negative required capacity and retry with exact-size malloc() storage
  • keep typical metadata on the stack and free oversized returned storage only when it differs from the stack buffer
  • use the pointer-returning join_paths() helper for timing output
  • reject a missing or empty TMPDIR and close the output file on setup failure
  • remove timing and assembly-store strings.hh dependencies
  • update the affected CoreCLR APK size references from Azure DevOps build 1566719

Validation

  • Local native builds intentionally skipped; relying on CI validation
  • APK size references refreshed from Azure DevOps build 1566719

Copilot AI lite review requested due to automatic review settings August 25, 2026 11:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the shared native fast-timing implementation to stop using dynamic_local_string for formatting and option parsing, moving toward fixed-buffer (char[]) handling to reduce reliance on the local-string hierarchy and eventually C++ runtime dependencies in shared native code.

Changes:

  • Switch timing option retrieval/parsing (debug.mono.timing) to fixed buffers and in-place C-string tokenization.
  • Replace timing log/event message construction with fixed char arrays and snprintf, returning event descriptions as string literals.
  • Add AndroidSystem::monodroid_get_system_property overloads that write directly into caller-provided char[] buffers (MonoVM and CLR variants).
Show a summary per file
File Description
src/native/mono/runtime-base/android-system.hh Adds char[] overload for system property retrieval on the MonoVM side.
src/native/common/runtime-base/timing-internal.cc Migrates timing option parsing, log message formatting, and output path building to fixed buffers.
src/native/common/include/runtime-base/timing-internal.hh Updates timing message formatting helpers to use fixed buffers and snprintf.
src/native/clr/include/runtime-base/android-system.hh Adds char[] overload for system property retrieval on the CLR side.

Review details

  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/native/common/runtime-base/timing-internal.cc Outdated
Comment thread src/native/common/runtime-base/timing-internal.cc
@simonrozsival simonrozsival added the drop-libcpp Work to remove the libc++ dependency from Android NativeAOT label Aug 25, 2026
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/remove-timing-dynamic-strings branch from 6b89b87 to 1e2809e Compare August 25, 2026 15:06
@simonrozsival
simonrozsival changed the base branch from main to dev/simonrozsival/use-join-paths-for-logger August 25, 2026 15:10
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/remove-timing-dynamic-strings branch 2 times, most recently from 176c7d7 to 75a294a Compare August 25, 2026 15:29
@simonrozsival simonrozsival changed the title Use fixed buffers for timing strings Remove local strings from timing Aug 25, 2026
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/remove-timing-dynamic-strings branch 9 times, most recently from d9fb4b3 to f02f1d2 Compare August 25, 2026 22:23
simonrozsival and others added 10 commits August 26, 2026 09:34
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The fixed timing buffers reduce libmonodroid.so from 1,184,800 to 1,094,848 bytes. Refresh all four affected CoreCLR package baselines using build 1566719 output.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assembly store timing metadata no longer uses the local-string types.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Allocate timing metadata to its exact size instead of treating the former local-string stack threshold as a maximum.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use malloc only when assembly timing information exceeds the former local-string stack threshold.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Retry timing metadata formatting with exact heap storage only when the sensible stack buffer is insufficient.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route assembly timing heap-buffer cleanup through the same helper as temporary path buffers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rely on free(nullptr) and name the stack-backed timing buffer explicitly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Eliminate the separate heap pointer and free assembly timing storage only when it differs from the stack buffer.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/remove-timing-dynamic-strings branch from f02f1d2 to 17bbadb Compare August 26, 2026 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drop-libcpp Work to remove the libc++ dependency from Android NativeAOT

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants